Use stable PostHog identity for newsletter signups#6588
Conversation
Greptile SummaryThis PR stops using email addresses as PostHog distinct IDs for demo/intro form tracking and newsletter signups. It introduces a shared
Confidence Score: 4/5Safe to merge; the only actionable finding is that The core identity-management logic is well-constructed and the JS helper correctly isolates PII into person properties. The one concrete gap is in
Important Files Changed
Reviews (1): Last reviewed commit: "Use stable PostHog identity for newslett..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| yield track_newsletter_posthog_subscription(email) | ||
| yield IndexState.send_contact_to_webhook(email) | ||
| yield IndexState.add_contact_to_loops(email) |
There was a problem hiding this comment.
newsletter_subscribed event even when email is None. If the user submits the form without an email address, email_to_validate is falsy and the if block is skipped entirely — so email stays None and tracking fires with an empty props object. The resulting event has no identity and no person properties, creating noise in PostHog analytics.
| yield track_newsletter_posthog_subscription(email) | |
| yield IndexState.send_contact_to_webhook(email) | |
| yield IndexState.add_contact_to_loops(email) | |
| if email: | |
| yield track_newsletter_posthog_subscription(email) | |
| yield IndexState.send_contact_to_webhook(email) | |
| yield IndexState.add_contact_to_loops(email) |
| if (Object.keys(personProps).length > 0) {{ | ||
| eventProps.$set = personProps; | ||
| }} | ||
| if (canonicalDistinctId && canonicalDistinctId !== props.email) {{ | ||
| posthog.identify(String(canonicalDistinctId), personProps); | ||
| }} |
There was a problem hiding this comment.
When canonicalDistinctId is truthy, posthog.identify(canonicalDistinctId, personProps) is called and then posthog.capture(event_name, {..., $set: personProps}) is called immediately after with the same personProps in $set. PostHog merges both writes, so this is harmless in practice, but the $set payload on the capture event is redundant when identify already carries the same data. Removing the $set attachment when canonicalDistinctId is present would eliminate the duplicate write, though this is a minor analytics hygiene concern.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
$setperson propertiesposthog.identifywhen a canonical non-email ID is providedcc @Alek99
Testing
uv run ruff format packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/posthog.py packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/__init__.py packages/reflex-site-shared/src/reflex_site_shared/backend/signup.pyuv run ruff check packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/posthog.py packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/__init__.py packages/reflex-site-shared/src/reflex_site_shared/backend/signup.pyuv run --package reflex-site-shared python - <<'PY' ...script assertions for newsletter PostHog payload